Fix MNR noise tracking and redundant activation control - #4807

Pull request overview

This pull request addresses weak stationary-noise suppression in the macOS minimum-statistics noise reducer (MNR) by correcting the estimator and decision-directed SNR recurrence, and simplifies the UI by removing the redundant “Enable MNR” checkbox so the DSP method selector is the sole enable/disable control.

Changes:

• Fixes MNR noise-floor tracking and decision-directed update logic (smoothed periodograms, calibrated min-statistics bias, controlled upward noise rise, correct posterior-SNR recurrence, and strength blend applied only at synthesis).
• Removes the legacy “Enable MNR” checkbox and its dead settings/signal wiring; retains the strength slider with clarified “0 = bypass”.
• Adds a deterministic macOS-only regression/quality test target and wires it into CTest.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Solid work, and the diagnosis is right on all three counts — the raw 25-frame minimum really was under-reporting stationary noise, m_prevPow really was feeding raw FFT power into a dimensionless recurrence, and folding the strength blend back into m_prevGain really did contaminate the adaptive state. Storing the previous posterior SNR and keeping m_filterGain strength-free are both textbook-correct now, and moving the blend into synthesizeFrameWithCurrentGain() is the right seam. I traced the UI removal through the tree: nothing reads the flat MnrEnabled key (persistence goes through ClientMnrEnabled), the method selector at AetherDspWidget.cpp:378 already calls setMnrEnabled(), the non-macOS gate at line 239 still carries the "macOS only" tooltip, and the new resetCurrentTab behaviour now matches every other tab. Clean removal. The new deterministic test target is a genuinely good addition — CMake placement matches its if(APPLE) siblings, and all four checks are green.

My one substantive question is about tuning rather than structure: OVER and FLOOR were almost certainly chosen to compensate for the estimator that this PR just fixed, and the numbers in your own test plan suggest they're now double-correcting.

Would like addressed before merge

• MacNRFilter.h:71 — OVER = 2.0 on top of a now-calibrated noise floor pins the mask near FLOOR for any stationary noise; worth a deliberate answer rather than leaving it implicit.

Polish

• tests/mac_nr_filter_test.cpp:199 — the desired-signal-loss assertion has 0.18 dB of headroom against your own measurement; likely to flake on a different Accelerate build.

Non-blocking notes

• src/core/MacNRFilter.cpp:144 — the zero-frame poisoning you fixed at startup can still recur mid-stream after a silence gap; the m_noiseInitialized seed only covers frame zero.
• The class doc block at the top of MacNRFilter.h (outside the diff) still describes the old design — "25-frame noise history … vs 10 frames before", no mention of the smoothed periodogram, bias calibration, or rise limiting. Worth a refresh while you're in there, since that comment is the first thing the next reader hits.

_{🤖 aethersdr-agent · cost: $8.2592 · model: claude-opus-5}

Nice piece of DSP work — the core diagnosis holds up on inspection. The decision-directed recurrence is now the textbook form (ξ = α·G²·γ_prev + (1-α)·max(γ-1,0)), where the old code was feeding raw FFT power in for γ_prev, which made the estimator scale-dependent; and pulling the strength blend out of m_filterGain so it only lands in synthesis is the right separation. I also confirmed the claim about the dead setting: nothing reads the flat MnrEnabled key — the persisted state is ClientMnrEnabled, written by MainWindow.cpp:3630 and read at MainWindow_Wiring.cpp:1677 — and the method selector (AetherDspWidget.cpp:378) already owns enable/disable, so the checkbox really was redundant. No stale Enable MNR references remain anywhere in the tree, and the new <algorithm> include in the header fixes a genuine transitive-include dependency for std::clamp in setStrength().

Nothing here blocks merge. Two estimator-seeding edge cases and a CI-coverage question are worth a look.

Polish

• Seeding m_noiseEst at the full power of the first energetic frame means enabling MNR (or a reset() on band change) mid-signal briefly suppresses wanted audio rather than passing it through — inline.
• The > 0.0f energy test accepts a single denormal, so a near-silent-but-dithered first frame can seed a ~zero floor that takes the history flush plus the NOISE_RISE ramp to recover from — inline, with a suggestion.

Non-blocking notes

• mac_nr_filter_test is registered only under if(APPLE) — inline; worth confirming the macOS CI job actually builds it, given the local linker crash noted in the test plan.
• m_frameCount (MacNRFilter.cpp:275, .h:97) is incremented and reset but never read — pre-existing dead state, and this PR is already tidying the estimator fields, so it'd be a natural thing to drop while you're in here. Outside the diff, hence not inline.

_{🤖 aethersdr-agent · cost: $4.9903 · model: claude-opus-5}

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

    for (int frame = 0; frame < kResetFrames; frame += kBlockFrames) {
        QByteArray block(kBlockFrames * 2 * static_cast<int>(sizeof(float)),
                         Qt::Uninitialized);
        std::copy_n(input.data() + 2 * frame, kBlockFrames * 2,
                    reinterpret_cast<float*>(block.data()));
        const QByteArray processed = filter.process(block);
        const auto* values = reinterpret_cast<const float*>(processed.constData());
        output.insert(output.end(), values,
                      values + processed.size() / static_cast<int>(sizeof(float)));
    }

Reviewed final state at 590c06e: all prior review points addressed in code (documented OVER rationale, -4 dB loss guard, MIN_FRAME_POWER silence freeze, 0.25 conservative seed, macOS CI runs mac_nr_filter_test, dead m_frameCount dropped). DSP recurrence and strength/mask separation verified; no stale MNR checkbox references remain. All checks green.